Allow drivers to supply a list of extra symbols to intern #138682

Alexendoo · 2025-03-19T02:54:53Z

Allows adding new symbols as consts in external drivers, desirable in Clippy so we can use them in patterns to replace code like

rust/src/tools/clippy/clippy_lints/src/casts/cast_ptr_alignment.rs

Line 66 in 75530e9

if matches!(name.ident.as_str(), "read_unaligned" | "write_unaligned")

The Clippy change adds a couple symbols as a demo, the exact clippy_utils API and replacing other usages can be done on the Clippy side to minimise sync conflicts

try-job: aarch64-gnu

rustbot · 2025-03-19T02:54:57Z

r? @fee1-dead

rustbot has assigned @fee1-dead.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

rustbot · 2025-03-19T03:45:35Z

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

blyxyas · 2025-03-19T14:47:42Z

This will help a lot when having symbols on Clippy. We usually just batched these and submitted a PR to add 3 or 4 symbols at a time.

Strong +1 ! 👀

bors · 2025-03-20T08:36:16Z

☔ The latest upstream changes (presumably #137930) made this pull request unmergeable. Please resolve the merge conflicts.

fee1-dead · 2025-03-24T11:06:51Z

compiler/rustc_macros/src/symbols.rs

-                Interner::prefill(&[
-                    #prefill_stream
-                ])
+            pub(crate) fn fresh(extra: &[&'static str]) -> Self {


I would recommend keeping fresh() but naming a new method for this. Perhaps new_with_extra_preinterned_symbols?

Renamed it to with_extra_symbols and added some docs

There's only one callsite so the current fresh would be unused

fee1-dead · 2025-03-24T11:09:00Z

compiler/rustc_macros/src/symbols.rs

@@ -298,7 +298,7 @@ fn symbols_with_errors(input: TokenStream) -> (TokenStream, Vec<syn::Error>) {
    let preinterned_symbols_count = entries.len();
    let output = quote! {
        const SYMBOL_DIGITS_BASE: u32 = #symbol_digits_base;
-        const PREINTERNED_SYMBOLS_COUNT: u32 = #preinterned_symbols_count;
+        pub const PREINTERNED_SYMBOLS_COUNT: u32 = #preinterned_symbols_count;


This needs a better name and we should document what it really means. Perhaps something like

Suggested change

pub const PREINTERNED_SYMBOLS_COUNT: u32 = #preinterned_symbols_count;

/// The number of predefined symbols; this is the the first index for

/// extra pre-interned symbols in an Interner created via

/// `new_with_extra_preinterned_symbols`.

pub const PREDEFINED_SYMBOLS_COUNT: u32 = #predefined_symbols_count;

fee1-dead

Sorry for the delay! I saw that this was still S-waiting-on-author so I assumed that I didn't need to review this. For next time, it would be really nice if you could do @rustbot ready to let me know when this is ready.

I had some changes that I wanted to make, but overall I am happy enough for this to land. I'll work on the followup.

fee1-dead · 2025-04-09T09:03:58Z

compiler/rustc_span/src/symbol.rs

@@ -2730,8 +2728,8 @@ impl Symbol {
    }

    /// Is this symbol was interned in compiler's `symbols!` macro


Suggested change

/// Is this symbol was interned in compiler's `symbols!` macro

/// Was this symbol predefined in the compiler's `symbols!` macro

fee1-dead · 2025-04-09T09:04:30Z

compiler/rustc_middle/src/query/on_disk_cache.rs

        // if symbol preinterned, emit tag and symbol index
-        if symbol.is_preinterned() {
+        if symbol.is_predefined() {
            self.encoder.emit_u8(SYMBOL_PREINTERNED);


should be changed to predefined too.

fee1-dead · 2025-04-09T09:08:07Z

@bors r+ rollup

bors · 2025-04-09T09:08:10Z

📌 Commit abe6e88 has been approved by fee1-dead

It is now in the queue for this repository.

Allow drivers to supply a list of extra symbols to intern Allows adding new symbols as `const`s in external drivers, desirable in Clippy so we can use them in patterns to replace code like https://github.com/rust-lang/rust/blob/75530e9f72a1990ed2305e16fd51d02f47048f12/src/tools/clippy/clippy_lints/src/casts/cast_ptr_alignment.rs#L66 The Clippy change adds a couple symbols as a demo, the exact `clippy_utils` API and replacing other usages can be done on the Clippy side to minimise sync conflicts

Rollup of 18 pull requests Successful merges: - rust-lang#137412 (Ensure `swap_nonoverlapping` is really always untyped) - rust-lang#138167 (Small code improvement in rustdoc hidden stripper) - rust-lang#138605 (Clean up librustdoc::html::render to be better encapsulated) - rust-lang#138682 (Allow drivers to supply a list of extra symbols to intern) - rust-lang#138904 (Test linking and running `no_std` binaries) - rust-lang#139423 (Suppress missing field error when autoderef bottoms out in infer) - rust-lang#139449 (match ergonomics: replace `peel_off_references` with a recursive call) - rust-lang#139507 (compiletest: Trim whitespace from environment variable names) - rust-lang#139530 (Remove some dead or leftover code related to rustc-intrinsic abi removal) - rust-lang#139560 (fix title of offset_of_enum feature) - rust-lang#139563 (emit a better error message for using the macro incorrectly) - rust-lang#139568 (Don't use empty trait names) - rust-lang#139580 (Temporarily leave the review rotation) - rust-lang#139589 (saethlin is back from vacation) - rust-lang#139592 (rustdoc: Enable Markdown extensions when looking for doctests) - rust-lang#139599 (Tracking issue template: fine-grained information on style update status) - rust-lang#139600 (Update `compiler-builtins` to 0.1.153) - rust-lang#139606 (Update compiletest to Edition 2024) r? `@ghost` `@rustbot` modify labels: rollup

Allow drivers to supply a list of extra symbols to intern Allows adding new symbols as `const`s in external drivers, desirable in Clippy so we can use them in patterns to replace code like https://github.com/rust-lang/rust/blob/75530e9f72a1990ed2305e16fd51d02f47048f12/src/tools/clippy/clippy_lints/src/casts/cast_ptr_alignment.rs#L66 The Clippy change adds a couple symbols as a demo, the exact `clippy_utils` API and replacing other usages can be done on the Clippy side to minimise sync conflicts

Rollup of 17 pull requests Successful merges: - rust-lang#137412 (Ensure `swap_nonoverlapping` is really always untyped) - rust-lang#138167 (Small code improvement in rustdoc hidden stripper) - rust-lang#138605 (Clean up librustdoc::html::render to be better encapsulated) - rust-lang#138682 (Allow drivers to supply a list of extra symbols to intern) - rust-lang#138904 (Test linking and running `no_std` binaries) - rust-lang#139423 (Suppress missing field error when autoderef bottoms out in infer) - rust-lang#139449 (match ergonomics: replace `peel_off_references` with a recursive call) - rust-lang#139507 (compiletest: Trim whitespace from environment variable names) - rust-lang#139530 (Remove some dead or leftover code related to rustc-intrinsic abi removal) - rust-lang#139560 (fix title of offset_of_enum feature) - rust-lang#139563 (emit a better error message for using the macro incorrectly) - rust-lang#139568 (Don't use empty trait names) - rust-lang#139580 (Temporarily leave the review rotation) - rust-lang#139589 (saethlin is back from vacation) - rust-lang#139592 (rustdoc: Enable Markdown extensions when looking for doctests) - rust-lang#139599 (Tracking issue template: fine-grained information on style update status) - rust-lang#139600 (Update `compiler-builtins` to 0.1.153) r? `@ghost` `@rustbot` modify labels: rollup

Zalathar · 2025-04-10T09:19:04Z

Failed in rollup: #139613 (comment)

@bors r-

fee1-dead · 2025-04-10T09:59:08Z

Well then, please address my review comments and fix the test failure when you have time :)

Alexendoo · 2025-04-10T13:56:31Z

@rustbot ready

Zalathar · 2025-04-10T14:05:07Z

@bors try

bors · 2025-04-10T14:06:20Z

⌛ Trying commit f740326 with merge 1fd0466b59234905a1f4f191166204db03bcf7cb...

Allow drivers to supply a list of extra symbols to intern Allows adding new symbols as `const`s in external drivers, desirable in Clippy so we can use them in patterns to replace code like https://github.com/rust-lang/rust/blob/75530e9f72a1990ed2305e16fd51d02f47048f12/src/tools/clippy/clippy_lints/src/casts/cast_ptr_alignment.rs#L66 The Clippy change adds a couple symbols as a demo, the exact `clippy_utils` API and replacing other usages can be done on the Clippy side to minimise sync conflicts --- try-job: aarch64-gnu

bors · 2025-04-10T16:01:18Z

☀️ Try build successful - checks-actions
Build commit: 1fd0466 (1fd0466b59234905a1f4f191166204db03bcf7cb)

rustbot assigned fee1-dead Mar 19, 2025

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Mar 19, 2025

This comment has been minimized.

Sign in to view

Alexendoo force-pushed the extra-symbols branch from d0ed3d0 to 6547c4d Compare March 19, 2025 03:45

This comment has been minimized.

Sign in to view

Alexendoo force-pushed the extra-symbols branch from 6547c4d to 8ab6d63 Compare March 19, 2025 14:13

Alexendoo force-pushed the extra-symbols branch from 8ab6d63 to d12c235 Compare March 20, 2025 14:04

fee1-dead requested changes Mar 24, 2025

View reviewed changes

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 24, 2025

Alexendoo force-pushed the extra-symbols branch from d12c235 to abe6e88 Compare March 24, 2025 15:45

fee1-dead approved these changes Apr 9, 2025

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 9, 2025

Zalathar mentioned this pull request Apr 10, 2025

Rollup of 18 pull requests #139612

Closed

Zalathar mentioned this pull request Apr 10, 2025

Rollup of 17 pull requests #139613

Closed

bors removed the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Apr 10, 2025

bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Apr 10, 2025

Allow drivers to supply a list of extra symbols to intern

f740326

Alexendoo force-pushed the extra-symbols branch from abe6e88 to f740326 Compare April 10, 2025 13:55

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 10, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow drivers to supply a list of extra symbols to intern #138682

Allow drivers to supply a list of extra symbols to intern #138682

Alexendoo commented Mar 19, 2025 •

edited by Zalathar

Loading

rustbot commented Mar 19, 2025

This comment has been minimized.

rustbot commented Mar 19, 2025

This comment has been minimized.

blyxyas commented Mar 19, 2025

bors commented Mar 20, 2025

fee1-dead Mar 24, 2025

Alexendoo Mar 24, 2025

fee1-dead Mar 24, 2025

fee1-dead left a comment

fee1-dead Apr 9, 2025

fee1-dead Apr 9, 2025

fee1-dead commented Apr 9, 2025

bors commented Apr 9, 2025

Zalathar commented Apr 10, 2025

fee1-dead commented Apr 10, 2025

Alexendoo commented Apr 10, 2025

Zalathar commented Apr 10, 2025

bors commented Apr 10, 2025

bors commented Apr 10, 2025

-        pub const PREINTERNED_SYMBOLS_COUNT: u32 = #preinterned_symbols_count;
+        /// The number of predefined symbols; this is the the first index for
+        /// extra pre-interned symbols in an Interner created via
+        /// `new_with_extra_preinterned_symbols`.
+        pub const PREDEFINED_SYMBOLS_COUNT: u32 = #predefined_symbols_count;

		@@ -2730,8 +2728,8 @@ impl Symbol {
		}

		/// Is this symbol was interned in compiler's `symbols!` macro

	/// Is this symbol was interned in compiler's `symbols!` macro
	/// Was this symbol predefined in the compiler's `symbols!` macro

Allow drivers to supply a list of extra symbols to intern #138682

Are you sure you want to change the base?

Allow drivers to supply a list of extra symbols to intern #138682

Conversation

Alexendoo commented Mar 19, 2025 • edited by Zalathar Loading

rustbot commented Mar 19, 2025

This comment has been minimized.

rustbot commented Mar 19, 2025

This comment has been minimized.

blyxyas commented Mar 19, 2025

bors commented Mar 20, 2025

fee1-dead Mar 24, 2025

Choose a reason for hiding this comment

Alexendoo Mar 24, 2025

Choose a reason for hiding this comment

fee1-dead Mar 24, 2025

Choose a reason for hiding this comment

fee1-dead left a comment

Choose a reason for hiding this comment

fee1-dead Apr 9, 2025

Choose a reason for hiding this comment

fee1-dead Apr 9, 2025

Choose a reason for hiding this comment

fee1-dead commented Apr 9, 2025

bors commented Apr 9, 2025

Zalathar commented Apr 10, 2025

fee1-dead commented Apr 10, 2025

Alexendoo commented Apr 10, 2025

Zalathar commented Apr 10, 2025

bors commented Apr 10, 2025

bors commented Apr 10, 2025

Alexendoo commented Mar 19, 2025 •

edited by Zalathar

Loading